hvm: Fix undefined bit shifting in mmio emulation path

In functions set_eflags_* (xen/arch/x86/hvm/io.c), if the first
argument "size" equals sizeof(long), the following code will produce
unintended and invalid result:
        unsigned long mask = (1 << (8 * size)) - 1;

In ANSI C, if the shift amount is greater or equal to the width of the
data type, the result is undefined. Specifically on x86, a bit mask is
applied to the shift amount, so that more significant bits are
ignored. So the above expression results 0x0 instead of the intended
~0UL.

This patch fixes this issue. Because size=0 is not a valid parameter,
rewriting the code using right shift avoids an additional condition
check.

Signed-off-by: Qing He <qing.he@intel.com>

diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index f82c28e82af2285200dd9b0b2722a495c0b48a81..b9f0a39c2c023a5f425e5c7bbe0d383acee54639 100644 (file)
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -292,7 +292,11 @@ extern long get_reg_value(int size, int index, int seg, struct cpu_user_regs *re
 static inline void set_eflags_CF(int size, unsigned long v1,
                                  unsigned long v2, struct cpu_user_regs *regs)
 {
-    unsigned long mask = (1 << (8 * size)) - 1;
+    unsigned long mask;
+    
+    ASSERT((size <= sizeof(mask)) && (size > 0));
+
+    mask = ~0UL >> (8 * (sizeof(mask) - size));
 
     if ((v1 & mask) > (v2 & mask))
         regs->eflags |= X86_EFLAGS_CF;
@@ -303,7 +307,13 @@ static inline void set_eflags_CF(int size, unsigned long v1,
 static inline void set_eflags_OF(int size, unsigned long v1,
                                  unsigned long v2, unsigned long v3, struct cpu_user_regs *regs)
 {
-    if ((v3 ^ v2) & (v3 ^ v1) & (1 << ((8 * size) - 1)))
+    unsigned long mask;
+
+    ASSERT((size <= sizeof(mask)) && (size > 0));
+
+    mask = ~0UL >> (8 * (sizeof(mask) - size));
+    
+    if ((v3 ^ v2) & (v3 ^ v1) & mask)
         regs->eflags |= X86_EFLAGS_OF;
 }
 
@@ -317,7 +327,11 @@ static inline void set_eflags_AF(int size, unsigned long v1,
 static inline void set_eflags_ZF(int size, unsigned long v1,
                                  struct cpu_user_regs *regs)
 {
-    unsigned long mask = (1 << (8 * size)) - 1;
+    unsigned long mask;
+    
+    ASSERT((size <= sizeof(mask)) && (size > 0));
+
+    mask = ~0UL >> (8 * (sizeof(mask) - size));
 
     if ((v1 & mask) == 0)
         regs->eflags |= X86_EFLAGS_ZF;
@@ -326,7 +340,13 @@ static inline void set_eflags_ZF(int size, unsigned long v1,
 static inline void set_eflags_SF(int size, unsigned long v1,
                                  struct cpu_user_regs *regs)
 {
-    if (v1 & (1 << ((8 * size) - 1)))
+    unsigned long mask;
+    
+    ASSERT((size <= sizeof(mask)) && (size > 0));
+
+    mask = ~0UL >> (8 * (sizeof(mask) - size));
+
+    if (v1 & mask)
         regs->eflags |= X86_EFLAGS_SF;
 }